Skip to content

feature/admin-location-ms-access#343

Merged
jirhiker merged 1 commit into
stagingfrom
feature/admin-location-ms-access
Jan 7, 2026
Merged

feature/admin-location-ms-access#343
jirhiker merged 1 commit into
stagingfrom
feature/admin-location-ms-access

Conversation

@jirhiker

@jirhiker jirhiker commented Jan 7, 2026

Copy link
Copy Markdown
Member

Why

This PR addresses the following problem / context:

  • Use bullet points here

How

Implementation summary - the following was changed / added / removed:

  • Use bullet points here

Notes

Any special considerations, workarounds, or follow-up work to note?

  • Use bullet points here

@jirhiker jirhiker merged commit 3632f55 into staging Jan 7, 2026
5 of 6 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 590858e772

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread core/initializers.py


def register_routes(app):
from admin.auth_routes import router as admin_auth_router

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Provide admin.auth_routes before importing it

The new register_routes import expects an admin.auth_routes module, but there is no such file in the repo (only this reference shows up in rg). When register_routes runs at startup, Python will raise ModuleNotFoundError, preventing the app from booting in any environment where routes are registered.

Useful? React with 👍 / 👎.

Comment thread admin/auth.py
Comment on lines 242 to +245
# Store original URL to redirect back after login
original_url = str(request.url_for("admin:index"))
request.session["auth_redirect"] = original_url

# Redirect to Authentik login
return RedirectResponse(url=authentik_authorize_url, status_code=302)

async def logout(self, request: Request) -> RedirectResponse:
redirect_uri = str(request.url_for("admin_auth_callback"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Ensure admin_auth_callback route exists for url_for

The login flow now calls request.url_for("admin_auth_callback"), but there is no route with that name defined anywhere in the codebase (the only match is this line). Starlette raises NoMatch when url_for targets an unknown route, so any admin login attempt will error before redirecting to Authentik unless a matching callback route is added.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant